Problem: no command to patch missing transactions between v0.7.0 and v0.7.1 #513
Conversation
yihuang
requested review from
calvinaco and
leejw51crypto
and removed request for
a team
…v0.7.1 Solution: - add fix-unlucky-tx command to patch txs post v0.7.0 upgrade.
| cmd := &cobra.Command{ | ||
| Use: "fix-unlucky-tx [blocks-file]", | ||
| Short: "Fix tx execution result of false-failed tx after v0.7.0 upgrade, \"-\" means stdin.", | ||
| Long: "Fix tx execution result of false-failed tx after v0.7.0 upgrade.\nWARNING: don't use this command to patch blocks generated before v0.7.0 upgrade", |
There was a problem hiding this comment.
as a safe guard, would it be better to hardcode the upgrade height number and skip the command if the height is below this number? (or have two number, one for testnet-x, one for mainnet)
There was a problem hiding this comment.
that would complicate the code a little bit though.
To do conditional compile with network build tag, we must create separate modules right?
There was a problem hiding this comment.
maybe just one for mainnet + have a --force flag to skip the safe guard?
There was a problem hiding this comment.
hmm since we specify the chainId, maybe a simple map chainId -> number would be enough?
There was a problem hiding this comment.
hmm, maybe a default value for mainnet, but user can supply a different value?
There was a problem hiding this comment.
what would happen if someone try to patch a tx before v0.7.0 upgrade? can it leads to possible state corruption?
There was a problem hiding this comment.
added a --min-block-height as safty guard, with default value of 2693800.
There was a problem hiding this comment.
from your comment below, I understand that doing it on block before v0.7.0 could lead to state corruption because of the event format?
in that case, having the ability to override --min-block-height could be dangerous if bad interpreted, unless there is a reason for node operator to use it?
There was a problem hiding this comment.
maybe at least, strongly enforce the min-block-height when chain-id cronosmainnet_25-1 is specified
There was a problem hiding this comment.
we'll provide an external doc to the user: https://github.com/crypto-org-chain/cronos/wiki/Patch-Unlucky-Tx#post-v070-upgrade, the users should simply follow the procedures in the doc.
| # wait for tm-rpc port | ||
| wait_for_port(ports.rpc_port(custom_cronos.base_port(0))) | ||
| # check the tx indexer | ||
| assert len(cli.txs(f"ethereum_tx.ethereumTxHash={txhashes[1].hex()}")["txs"]) == 1 |
There was a problem hiding this comment.
This integration test only works before the root cause is fixed: #503, after that, there's nothing to patch.
Solution:
How To Use
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make)make test)go fmt)golangci-lint run)go list -json -m all | nancy sleuth)Thank you for your code, it's appreciated! :)